fix(handler): prevent double slash in URLs for root path prefix #487
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes a double slash issue in URLs when using an empty/root path prefix. The problem occurred because NormalizePathPrefix("") returned "/", which when concatenated with /api/... resulted in //api/..., causing unnecessary 301 redirects.
Changes:
- Modified
NormalizePathPrefixto return empty string for root paths (""or"/") instead of"/" - Updated URL construction and conditional checks throughout the codebase to work with the new normalization behavior
- Fixed a pre-existing bug where
NormalizePathPrefixwould return without trimming trailing slashes when adding a leading slash
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| handler.go | Updated NormalizePathPrefix function to return empty string for root paths, simplified prefix assignment after normalization, updated middleware condition to check for empty string instead of "/", and fixed a bug in path prefix normalization logic |
| internal/riveruicmd/riveruicmd.go | Simplified health check URL construction by removing redundant TrimSuffix call since normalized prefix never has trailing slashes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bgentry
left a comment
bgentry
left a comment
There was a problem hiding this comment.
@boostvolt thank you for this and sorry for the delay! I needed some time to add test coverage and manually verify it. We'll have it shipped in the next release.
|
@boostvolt ah, I got ahead of myself, would you be able to add yourself to the CLA at https://github.com/riverqueue/rivercla ? |
|
@bgentry Done! Just signed the CLA (riverqueue/rivercla#17). Thanks for merging! |
2 participants
NormalizePathPrefix("")returns/for empty prefix, but URL construction appends/api/..., resulting in//api/health-checks/complete. This causes unnecessary 301 redirects on every health check.Now returning "" for empty/root prefix instead of
/.